perf: optimize parsing V3.1 documents#2748
Conversation
|
@microsoft-github-policy-service agree |
There was a problem hiding this comment.
Pull request overview
Optimizes OpenAPI v3.1/v3.2 schema parsing by avoiding JSON pointer location construction unless it’s needed for $ref handling, and extends the benchmark suite to include “GHES Next” OpenAPI descriptions to measure performance impact on larger v3.1 documents.
Changes:
- Lazily compute
ParsingContext.GetLocation()only when a schema$refpointer is present (v3.1/v3.2 schema deserializers). - Optimize
ParsingContext.GetLocation()implementation to reduce allocations (StringBuilder-based pointer construction). - Add GHES Next YAML/JSON benchmarks and update committed BenchmarkDotNet artifact reports.
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/Microsoft.OpenApi/Reader/V31/OpenApiSchemaDeserializer.cs | Avoids computing location unless $ref is present. |
| src/Microsoft.OpenApi/Reader/V32/OpenApiSchemaDeserializer.cs | Avoids computing location unless $ref is present. |
| src/Microsoft.OpenApi/Reader/ParsingContext.cs | Optimizes JSON pointer location string construction. |
| performance/benchmark/Descriptions.cs | Adds GHES Next YAML/JSON benchmarks and setup downloads. |
| performance/benchmark/BenchmarkDotNet.Artifacts/results/performance.EmptyModels-report.html | Updated benchmark output artifact. |
| performance/benchmark/BenchmarkDotNet.Artifacts/results/performance.EmptyModels-report.csv | Updated benchmark output artifact. |
| performance/benchmark/BenchmarkDotNet.Artifacts/results/performance.EmptyModels-report-github.md | Updated benchmark output artifact. |
| performance/benchmark/BenchmarkDotNet.Artifacts/results/performance.Descriptions-report.json | Updated benchmark output artifact (includes GHES Next results). |
| performance/benchmark/BenchmarkDotNet.Artifacts/results/performance.Descriptions-report.html | Updated benchmark output artifact (includes GHES Next results). |
| performance/benchmark/BenchmarkDotNet.Artifacts/results/performance.Descriptions-report.csv | Updated benchmark output artifact (includes GHES Next results). |
| performance/benchmark/BenchmarkDotNet.Artifacts/results/performance.Descriptions-report-github.md | Updated benchmark output artifact (includes GHES Next results). |
nikcio
force-pushed
the
feature/perf-parsing
branch
from
abd707e to
dccf812
Compare
baywet
left a comment
baywet
left a comment
There was a problem hiding this comment.
Thank you for making the changes!
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| foreach (char c in segment) | ||
| { | ||
| if (c == '~' || c == '/') | ||
| { | ||
| totalLength++; // Each escape adds one additional character | ||
| } | ||
| } |
Check notice
Code scanning / CodeQL
Missed opportunity to use Where Note
| } | ||
| finally | ||
| { | ||
| ArrayPool<char>.Shared.Return(rentedBuffer, clearArray: true); |
There was a problem hiding this comment.
ArrayPool<char>.Shared.Return(..., clearArray: true) will clear the entire rented array (often larger than totalLength), adding an O(n) cost on every GetLocation() call and potentially offsetting the perf gains. Since this buffer only holds transient JSON pointer text, consider returning without clearing (or, if you need to avoid retaining data, clear only the used slice [0..position) before returning).
| ArrayPool<char>.Shared.Return(rentedBuffer, clearArray: true); | |
| ArrayPool<char>.Shared.Return(rentedBuffer, clearArray: false); |
…llocation on hot path(#2748) * chore: Refresh benchmarks * test: Add GHES Next descriptions * perf: ParsingContext.GetLocation * perf: Only load node location when needed * perf: Update GetLocation implementation
fix: optimize parsing V3.1 documents by reducing GetLocation method allocation on hot path(#2748)
2 participants
Pull Request
Description
Optimizes
ParsingContext.GetLocation()and its usage inOpenApiSchemaDeserializeronly loading the location when needed.Also adds GHES Next descriptions to performance tests to measure impact in V3.1 documents.
Measured baseline before change:
Measured after change:
Type of Change
Related Issue(s)
Changes Made
ParsingContext.GetLocation()nodeLocationwhen needed inOpenApiSchemaDeserializerTesting
Checklist
Versions applicability
See the contributing guidelines for more information about how patches are applied across multiple versions.
Additional Notes